Looks great! Good effort.

Any challenges / general feedback on the process of writing this? I know O'Caml's a very complex language.

We added a hook to disable the hierarchy navigation fallback of jumping to the next delimiter, as this gives a poor navigation experience for OCaml code.

Sorry that explains the navigate into lists thing. You'll have to hoist that into the "declare" machinery and make it a formal toggle there then instead so it uses combobulate-get. It exists to keep it compatible with the default Emacs behaviour, but it'd be a useful thing to selectively toggle it.

Writen mostly by Tim McGilchrist and PizieDust (with some small cosmetic helps from Xavier Van de Woestyne)

By all means add your names next to the language in the README!

I do not see any edit tests for things like cloning. They are important because they use the movement machinery to select and edit. The snag here is that dumb things like whitespace and newlines are properly preserved; this is notoriously hard to get right without tests to ensure you do not break something in, you know, YAML and Go or something. Minor thing. Should be quick to generate by copy-pastaing how the existing tests do it.

I do not see any edit tests for things like cloning. They are important because they use the movement machinery to select and edit. The snag here is that dumb things like whitespace and newlines are properly preserved; this is notoriously hard to get right without tests to ensure you do not break something in, you know, YAML and Go or something. Minor thing. Should be quick to generate by copy-pastaing how the existing tests do it.

Thank you. Just updated it to have a test for cloning.

We added a hook to disable the hierarchy navigation fallback of jumping to the next delimiter, as this gives a poor navigation experience for OCaml code.

Sorry that explains the navigate into lists thing. You'll have to hoist that into the "declare" machinery and make it a formal toggle there then instead so it uses combobulate-get. It exists to keep it compatible with the default Emacs behaviour, but it'd be a useful thing to selectively toggle it.

Please can you give more clarification about what you mean by hoisting it into the declare machinery and adding the toggle feature?

Sure. The procedure-sibling, etc. - lacking a better word than 'declarative' - is where you declare what Combobulate does with a language. If you make a new declarative variable (see typescript for examples) and change the list machinery to use combobulate-get you remove the need to do setq-local.

You can probably do this also with the tree-sitter naming thing. I don't like the idea of having to wrap everything in complex name checking code everywhere - I feel this can be done centrally, in one place, without polluting random bits of code. But I haven't actually checked every code path.

Did another sweep. Aside from the minor issues mentioned -- and assuming all the tests pass, I am very sorry I have not checked -- we can get this merged once they're done. Let me know if you need advice with how to do this.

Thanks @mickeynp , So all the tests currently pass excellently:
Ran 1023 tests, 1023 results as expected, 0 unexpected (2026-03-31 06:49:33+0100, 5.151486 sec)

I'll be updating this PR soon with the changes according to your remarks

Sure. The procedure-sibling, etc. - lacking a better word than 'declarative' - is where you declare what Combobulate does with a language. If you make a new declarative variable (see typescript for examples) and change the list machinery to use combobulate-get you remove the need to do setq-local.

You can probably do this also with the tree-sitter naming thing. I don't like the idea of having to wrap everything in complex name checking code everywhere - I feel this can be done centrally, in one place, without polluting random bits of code. But I haven't actually checked every code path.

Some changes are proposed in #158

Thanks. Once the imenu thing is moved out we can get it all merged in.

Don't forget to update the README with your names if you feel like it.

Thanks. Once the imenu thing is moved out we can get it all merged in.

Thank you. Is the change proposed in #158 what you have in mind or do you want us to remove the imenu support for ocaml all together?

Our idea was that once #158 is merged, we can then update this PR

To be clear: the imenu treesit.el customisation is not something I want in Combobulate. it's major mode specific code and it belongs in whatever package uses it.

The other problem is the naming scheme of the tree-sitter symbols. You made a bunch of changes to combobulate to handle that. I do not like this approach. There should be one singular lookup in Combobulate for tree-sitter's language name if you do not wish to use the canonical one. (It will also need one or two tests to ensure it works.)

To be clear: the imenu treesit.el customisation is not something I want in Combobulate. it's major mode specific code and it belongs in whatever package uses it.

We have updated this PR removing imenu support and also updated #158 to only focus on navigate-down-into-list

Great. Thanks. I've merged it!

The other problem is the naming scheme of the tree-sitter symbols. You made a bunch of changes to combobulate to handle that. I do not like this approach. There should be one singular lookup in Combobulate for tree-sitter's language name if you do not wish to use the canonical one. (It will also need one or two tests to ensure it works.)

Hi @mickeynp, could you please explain this a bit more?

My understanding is that you want us to revert the modifications we made in other to support ocaml and ocaml_interface grammars?

Hi. I added a new comment just now.

Hi. I added a new comment just now.

I can't find the comment you added, could you link me to it

Oh that's weird.

https://github.com/mickeynp/combobulate/pull/157/changes#r3031991590

here you go

Oh that's weird.

https://github.com/mickeynp/combobulate/pull/157/changes#r3031991590

here you go

I think GitHub may be broken.
The link just goes to the diff section. I scrolled the entire diff and didn't find any comments.

Anyway. your changes in interface.el and elsewhere: any and all code that has been added that does additional tree-sitter language/name checking must be removed and put into one centralised function that is itself called ideally just one in the core.